Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pipelining API documentation #226

Merged
merged 15 commits into from
Jan 4, 2024
Merged

Add pipelining API documentation #226

merged 15 commits into from
Jan 4, 2024

Conversation

Dolu1990
Copy link
Member

@Dolu1990 Dolu1990 commented Nov 7, 2023

Copy link
Contributor

@dlmiles dlmiles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback

source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@andreasWallner andreasWallner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a bunch of nitpicks and typos

source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved

val PC = Payload(UInt(32 bits))
n0(PC) := 0x42
n0(PC, "true") := 0x42
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is the above two lines represent different signals?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes
n0_PC
n0_PC_true


* - API
- Description
* - node.setAlwaysValid()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have setIdle and setBlocked in Stream, if we need to provide a setAlwaysXXXX style function to keep consistency to lower study curve.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't realy the same use case.
setAlwaysValid isn't just valid := True, but also mean that the valid signal is always high in a static way, will never be False ever.
while setIdle is used to provide a value which can be overriden.

Still any sugestion for better API name is welcome :D

val c01 = StageLink(n0, n1)


S2mLink
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there are three types of link to represent the different Link strategy. How about add an apply function for the Link class and create different XXXLink by parameters. So that the code can be unchanged while Link type changing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, this can be added as an tool on the top of the existing API. I didn't focussed much on that part of things yet

source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
* - throwWhen(Bool)
- Allows to cancel the current transaction from the pipeline (clear down.valid and c the transaction driver)
* - forgetOneWhen(Bool)
- Allows to request the upstream to forget the current transaction (but doesn't clear the down.valid)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the forget mean here, it will cancel the upstream's current transaction?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i'm replacing "the current" by "its current"

- Allows to cancel the current transaction from the pipeline (clear down.valid and c the transaction driver)
* - forgetOneWhen(Bool)
- Allows to request the upstream to forget the current transaction (but doesn't clear the down.valid)
* - ignoreReadyWhen(Bool)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is hard to understand for me. What the purpose for this? I guess it is to hang on the downstream transaction and fire upstream's?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgetOneWhen ?
It will not hang anything, just make the upstream transaction driver "forget" about its current state.
That's a light weight to flush / remove something from the pipeline without having to manipulate the ready wire

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgetOneWhen ? It will not hang anything, just make the upstream transaction driver "forget" about its current state. That's a light weight to flush / remove something from the pipeline without having to manipulate the ready wire

i mean the ignoreReadyWhen

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh. it is to fire the upstream.
It doesn't hang downstream

Its usage is to implement some logic to flush / remove elements of a pipeline.
You can do that for instance by combining ignoreReadyWhen and terminateWhen

* - ignoreReadyWhen(Bool)
- Allows to ignore the downstream ready (set up.ready)
* - duplicateWhen(Bool)
- Allows to duplicate the current transaction (clear up.ready)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean split one transaction into two?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it alows the transaction to eventualy duplicate downstream while holding a copy in the current node.

source/SpinalHDL/Libraries/Pipeline/introduction.rst Outdated Show resolved Hide resolved
@Dolu1990 Dolu1990 merged commit 747e879 into master Jan 4, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants